-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: usePreloaderInfo #8035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: usePreloaderInfo #8035
Conversation
|
GrandSchtroumpf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be a great addition. I think emitting event on internal task is a great way to expose new features with a low maintenance cost. I left some small comments
|
|
||
| const userEventPreloads = activePreloads.filter((item) => item.$inverseProbability$ <= 0.01); | ||
| window.dispatchEvent( | ||
| new CustomEvent('newPreloaderInfo', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rename it qpreload for consistency
| * | ||
| * @alpha | ||
| */ | ||
| export const usePreloaderInfo = (): { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I would not expose this as an public API, for maintenance purpose, but would let dev implement it as they want.
The event and its CustomEvent should be included in packages/qwik/src/core/render/jsx/types/jsx-qwik-attributes.ts to expose it inside useOnWindow
This could be added in the doc:
useOnWindow('newPreloaderInfo', $((ev) => {
const { userEventPreloads, activePreloads } = ev.details;
// Do something
}))| link.onload = link.onerror = () => { | ||
| link.onload = () => { | ||
| preloadCount--; | ||
| activePreloads.shift(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would keep the item just removed and send it in the event:
| activePreloads.shift(); | |
| const lastPreload = activePreloads.shift(); |
| const userEventPreloads = activePreloads.filter((item) => item.$inverseProbability$ <= 0.01); | ||
| window.dispatchEvent( | ||
| new CustomEvent('newPreloaderInfo', { | ||
| detail: { userEventPreloads, activePreloads }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also dispatch last preload. I know it could be deduce from the diff of previous and next activePreloads, but since we've got it in the this scope I think it would be an easy devX win.
| detail: { userEventPreloads, activePreloads }, | |
| detail: { userEventPreloads, activePreloads, lastPreload }, |
|
Just adding an example of how this feature could be leveraged: const LoadingStyle = () => {
useOnWindow('qpreload', sync$((e) => {
const selectors = e.details.activePreloads.map(({ url }) => `[on:click~=${url}]`);
const selectorStr = Array.from(new Set(selectors)).join(',');
const style = document.getElementById('loading-style');
style.innerHTML = `${selector} {cursor: wait}`
}));
return <style id="loading-style"></style>
}This will add a waiting cursor to all element that have |
What is it?
Description
Draft PR for QwikDev/qwik-evolution#311
Checklist
pnpm change